Skip to content

fix(core): refuse effect/listener/plugin registration while a fiber is UNLOADING - #96

Open
inso1337 wants to merge 1 commit into
cordiverse:mainfrom
inso1337:fix/assert-active-unloading
Open

fix(core): refuse effect/listener/plugin registration while a fiber is UNLOADING#96
inso1337 wants to merge 1 commit into
cordiverse:mainfrom
inso1337:fix/assert-active-unloading

Conversation

@inso1337

Copy link
Copy Markdown

Summary

Fiber.assertActive() only checked uid !== null — "not disposed", not
"not unloading". During a deactivation (a requirement is withdrawn and the
fiber survives as PENDING), an undo that calls ctx.effect(...) was
accepted, and the resulting disposer leaked permanently:

  1. the new effect executed and its disposer was pushed into
    fiber._disposablesafter _unload already clear()ed the list;
  2. the fiber ended PENDING while still holding an effect
    (fiber.getEffects().length > 0 on an inactive fiber);
  3. even a later fiber.dispose() never ran that disposer, because the epoch
    is already INACTIVE and no further unload is triggered — permanent
    residue
    .

Fix

The UNLOADING lifecycle check is added to a new assertRegistrable() used
by the registration entry points — effect(), on(), plugin() — and
NOT to assertActive() itself, because update() and restart() must keep
working through an inertial reload's UNLOADING pass (a broad guard broke
the existing "update config while injected service reloads" test).

assertRegistrable() {
  if (this.uid !== null && this.state !== FiberState.UNLOADING) return
  throw new CordisError('INACTIVE_EFFECT')
}

Repro / regression test

packages/core/tests/fiber.spec.ts — "refuses effect registration while
UNLOADING (G5 residue)": a provider dispose withdraws svc, the consumer's
generator-effect undo calls ctx.effect() during teardown. Before the fix
leaked === true and the fiber holds 1 leaked effect; after, leaked === false, getEffects().length === 0, state PENDING.

Full core suite: 71 passed.

Notes

…s UNLOADING

Fiber.assertActive() only checked uid !== null ('not disposed'), not
'not unloading'. During a deactivation (a requirement withdrawn, the fiber
survives as PENDING), an undo that calls ctx.effect() was accepted and the
resulting disposer leaked permanently: the new effect's disposer was pushed
after the unload snapshot was already taken, and no later dispose() ever ran
it — permanent residue (the G5 gap).

The UNLOADING check is NOT added to assertActive() itself, because update()
and restart() legitimately run through an inertial reload's UNLOADING pass.
Instead the guard is scoped to registration entry points:

- effect() -> assertRegistrable()
- events on() -> assertRegistrable()
- registry plugin() -> assertRegistrable()

assertRegistrable() throws INACTIVE_EFFECT when the fiber is disposed OR
unloading; update()/restart() keep the disposed-only check.

Regression test (fiber.spec.ts 'refuses effect registration while UNLOADING'):
a provider dispose withdraws 'svc', the consumer's generator-effect undo
calls ctx.effect() during teardown — leaks=false, effects=0, PENDING. Fails
before the fix (leaked=true), passes after. Full core suite: 71 passed.
inso1337 pushed a commit to inso1337/cordis that referenced this pull request Aug 24, 2026
…restart()

The broad assertActive() guard broke update() during an inertial reload
(which legitimately passes through UNLOADING) — the fork's existing
'update config while injected service reloads' test caught it. Move the
UNLOADING check to assertRegistrable(), used by effect()/on()/plugin();
update()/restart() keep the disposed-only check. Matches upstream PR
(cordiverse/cordis#96).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant